feat: delegate ready issues to stable agent subjects - #571

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Review Summary

🟡 Acceptable - The core architecture is sound: reusing the existing implementation prompt, checkout, commit, push, and PR publication helpers from main.py while adding profile-driven execution via worker.py is a clean extension. However, there are three material issues worth addressing before merge.

Findings

1. Missing AI disclosure on PR body (worker.py, publish method)

The publish method constructs the PR body inline as Closes #{issue["number"]}\n\nWritten by the issue-to-PR automation.\n... but does not include the required AI disclosure (_This pull request was opened by an AI agent (OpenHands)._). Every other PR-creation path in main.py routes through _pull_request_body -> _with_ai_disclosure, which appends this disclosure. The worker bypasses that function entirely. This is both a policy requirement (the repo AGENTS.md and external-services guidance mandate AI disclosure on human-readable content posted to GitHub) and a consistency issue.

2. Open PRs without software-factory/review status block all new issue processing (worker.py, run method)

When the prs list is non-empty (there are open PRs matching the branch prefix), the loop checks each PR software-factory/review commit status. If none have success, failure, or error status (e.g., the check has not run yet, was renamed, or was removed), the loop finishes with existing = None, and if existing is None: return exits the entire run without picking up new issues. A single open PR with a missing or stale check status thus blocks all new issue processing for that repository until the PR is closed or the check runs. Consider falling through to new-issue selection when no PR requires revision.

3. implement catches early-stop exceptions but publish still runs and may crash the run (worker.py, implement/publish)

When the agent stops early (AgentStopped or TimeoutError), implement catches the exception, writes a checkpoint, and comments on the issue. Control then falls through to publish, which calls _commit_agent_work. If the agent made no changes, publish raises RuntimeError("Implementation made no changes"), causing the automation run to fail with an error rather than exiting gracefully. The checkpoint comment says changes "will be submitted as a checkpoint," but if there are no changes, the run crashes instead. Consider having implement signal whether work was done, or having publish handle the no-changes case as a non-error when a checkpoint was already written.

Repository boundary

This PR is correctly scoped to the extensions registry: it adds a skill script, catalog manifest changes, documentation, and tests. The agent-profile dispatch behavior it depends on lives in OpenHands/automation#453 and OpenHands/software-agent-sdk#5010, which are the right repos for that behavior. No boundary concerns.

Testing

The contract test (test_github_developer_delivery.py) exercises the publish method with mocked workflow functions, verifying the call order (commit -> push -> PR -> comment). It does not test run() or implement(), so the blocking behavior and early-stop handling described above are untested. The test helper (github_automation_helpers.py) correctly loads the bundled files from the manifest, which is a good pattern.

[RISK ASSESSMENT]

• Overall PR: 🟡 MEDIUM
  • The blocking behavior (#2) could stall production issue processing under common conditions (a PR without the specific check status). The missing AI disclosure (#1) is a policy violation. The crash-on-no-changes (#3) causes noisy run failures. None are security issues, but all affect operational reliability.

VERDICT: ❌ Needs rework - Address the three findings above before merge. The AI disclosure gap and the blocking behavior are the most impactful.

KEY INSIGHT: The worker run() method treats any open PR as a gate on new work, but only a subset of PR states actually require revision - the rest should fall through to issue selection.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it is merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Review: feat: run issue-to-PR delivery with the selected agent profile

Repository boundary: This PR is correctly placed - it extends an existing skill/automation in the extensions registry and reuses the shared github_automation.py from the openhands-automation skill.

Critical Issue

Module-level import breaks the unprofiled workflow. The SKILL.md setup path (Step 8) packages only main.py - it does not include github_automation.py. But main.py now has from github_automation import github_request as _github_request at module level (line 353), which executes before main() is called. In the unprofiled workflow (no AUTOMATION_AGENT_PROFILE_ID), this import will raise ImportError: No module named 'github_automation' because the file isn't in the tarball. The PR description says "The existing unprofiled label workflow remains available," so this is a regression. The fix is to make the import lazy (move it into the functions that call _github_request) or update SKILL.md Step 8 to also package github_automation.py.

Improvement Opportunities

Per-repository error isolation is missing in profile mode. github_automation.main() iterates repositories with try/finally but no except. If automation.run() raises (e.g., the TimeoutError("Agent did not stop; checkpoint publication refused") from worker.py's implement()), the loop aborts and remaining repositories are never processed. The unprofiled workflow in main.py catches per-repo failures and continues. The profile path should do the same.

Test coverage is thin for the complexity added. The test covers only publish() with existing=None (happy path). The run() method contains ~60 lines of branching logic for PR selection, lane partitioning, dependency checking, base-update conflicts, and review-driven revision - none of which are tested. The checkpoint path in publish() (no commits + checkpoint.json exists) and the RuntimeError("Implementation made no changes") path are also untested.

Risk Assessment

• Overall PR: MEDIUM
• The module-level import is a blocking regression for the unprofiled workflow. The per-repo isolation gap means one stuck agent can block all repositories in a multi-repo deployment. Neither is a security issue, but both affect operational reliability.

Verdict: Needs rework - The import regression must be fixed before merge.

Key insight: The shared-module pattern is sound, but the import needs to be deferred so the unprofiled single-file deployment path keeps working.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with thumbs up or thumbs down to give feedback.

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

This PR extends the issue-to-PR automation with agent-profile-based execution, adding developer lanes, dependency checks, review-driven revisions, and conditional base updates. The architecture is sound: the existing main.py helpers (prompts, git, commit, push, PR publication) are reused via import, and worker.py adds the new orchestration layer on top of the shared GitHubAutomation base class. The test coverage exercises the key flows (new-issue publication, no-code revision, lane selection, failed-review revision, base refresh).

Findings

Two inline findings below. In summary:

1. Revision path prompt inconsistency (Medium): When revising an existing PR, _prepare_repository is called with branch as the clone source (line 201), so the workspace is a clone of the issue branch, not main. But _build_implementation_prompt is still called with base_branch from config (e.g. "main") and base set to the issue branch tip SHA. The prompt then tells the agent "It is a clone of main at {issue_branch_sha}" - factually incorrect. The revision prepend provides context, but the workspace description could mislead the agent about what code it has locally.

2. Silent return on 409 from update-branch (Low): The bare return on HTTP 409 (line 144) exits the entire run with no logging or comment. Other skip paths in the code use print(json.dumps(...)) for visibility. A 409 means the head SHA changed between listing and update - the next poll will retry, but there is zero observability into why the run did nothing.

Repository boundary check

This PR belongs in the extensions registry - it adds a skill script, automation catalog entry, and tests, all within the established skills/github-issue-to-pr/ and automations/catalog/ structure. The github_automation.py base class is imported from skills/openhands-automation/scripts/, which is also in this repo. No SDK behavior or agent-server code is introduced here.

Risk Assessment

• Overall PR: 🟡 MEDIUM
• The core logic is correct and well-tested for the happy paths. The prompt inconsistency in the revision path could confuse the agent but is mitigated by the revision prepend and the instruction to read PR state from GitHub directly. The silent 409 is a minor observability gap. Neither is a blocking issue.
• The security model is consistent with the documented profile-based approach: the agent has a write-capable GitHub token (necessary for reading PR reviews directly), but is instructed not to push. This is a deliberate trade-off from the unprofiled workflow where the agent has no token at all.

VERDICT: Worth merging. The two findings are improvement opportunities, not blockers.

KEY INSIGHT: The worker driver cleanly separates orchestration (lane selection, revision detection, base updates) from execution (prompts, git, publication) by reusing the existing main.py helpers, keeping the change focused and the existing unprofiled workflow untouched.

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.
   Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.
   Was this review helpful? React with 👍 or 👎 to give feedback.

Review of head 75fd841 — ✅ Approved

I reviewed the full diff against factory/issue-triage and ran the complete test suite (839 passed, 24 skipped). No blocking findings.

What this PR does

Introduces worker.py as the new catalog entrypoint for github-issue-to-pr, running issue delivery inside the conversation provisioned by the Automation Service's selected agent profile. The existing main.py remains the source of implementation prompts, Git helpers, and PR publication — worker.py adds readiness selection, dependency gating, PR-revision detection, and base-update handling on top of it.

Areas examined

worker.py (new, 259 lines)

• IssueToPR extends the shared GitHubRepository transport — no duplication of GitHub API code.
• run(): lane partitioning uses exact type(...) is int checks (correctly rejects bool), modulo-based issue ownership, and priority ordering (priority:high first, then issue number). PRs with the review label are skipped so a pending reviewer doesn't block the lane.
• Base SHA is fetched once per batch (not per PR), satisfying the earlier review feedback.
• update-branch handles 409 (head changed → skip with JSON log line and continue to next PR) and 422 (conflict → developer revision with conflict context in the prompt). Other HTTP errors are re-raised.
• implement(): interrupts a stuck agent, polls for a stopped state with a 30s deadline, writes a redacted checkpoint.json via workflow._redact(str(exc), self.token), and posts a generic reason (no error details or credentials leak into public comments).
• publish(): zero-commit revisions on an existing PR request another review via review_label without pushing; zero-commits on a new issue raises RuntimeError unless a checkpoint exists. PR body routes through workflow._pull_request_body → _with_ai_disclosure, so the AI disclosure is present.

main.py changes

• _prepare_repository now skips git checkout -b when branch == base_branch (revision path checks out the existing branch directly).
• _build_implementation_prompt gains publish_pr and github_access_instructions keyword-only params. When publish_pr=False, the agent summarizes instead of pushing/opening the PR — the coordinator owns publication. Access instructions are parameterized so the worker can pass the profile-scoped token name.
• The revision path passes checkout_branch (the issue branch) as base_branch to the prompt, so the workspace description ("clone of openhands/issue-4 at base") is now factually correct.

manifest.json

• Entrypoint → worker.py, version → 1.1.0, timeout → 3000s (matches the 2400s agent run + overhead). worker.py added to bundle files. New githubTokenSecret form field and agentProfiles feature requirement.

Tests (264 lines, 16 cases)

• Covers canonical publication reuse, no-code revision + review-label request, lane selection by priority (parametrized across main/release and pending-review states), failed-review revision with live feedback, base refresh independence from acceptance, checkpoint stop requirement (both outcomes), credential redaction in checkpoints vs. public comments, and base-update with head-change race. All use real code paths through the bundle helper with mocked GitHub/SDK boundaries.

Catalog integrity

• test_integration_catalog_in_sync, test_catalogs, test_catalog_schema, test_skills_catalog, and test_github_skill_installation all pass — the bundle index, manifest, and skill catalog are consistent.

Security

• Credentials are redacted via workflow._redact before writing checkpoints; raw error details and the token never appear in public comments (verified by test_checkpoint_keeps_error_details_out_of_public_comments).
• The token is referenced by name (--github-token-secret), never embedded in the bundle.
• github_instructions tells the agent to use GH_TOKEN=$TOKEN_NAME and never print the value.
• PR bodies include the AI disclosure via _pull_request_body → _with_ai_disclosure.

Previous review findings — all addressed

1. Missing AI disclosure on PR body → Fixed: publish() routes through workflow._pull_request_body.
2. Open PRs blocking new issue processing → Fixed: PRs with the review label are skipped; the loop falls through to new-issue selection.
3. implement fall-through crash on no changes → Fixed: zero-commits with an existing checkpoint returns gracefully.
4. Silent return on 409 → Fixed: now prints a JSON log line and continues to the next PR.
5. Raw exception text in checkpoint without redaction → Fixed: uses workflow._redact(str(exc), self.token).
6. Revision path prompt inconsistency → Fixed: checkout_branch is passed as base_branch, so the workspace description is accurate.

Minor notes (non-blocking)

• type(lane) is not int is intentional (rejects bool), though a one-line comment would help future readers.
• Conversation IDs are passed as plain text in the PR body rather than a clickable URL — this is a deliberate design decision (no public Canvas URL is available), as noted in earlier discussion.

No blocking findings. Approving.

Generated by OpenHands AI on behalf of the user.

Review of head 3764bac — ✅ Approved

I reviewed the full diff against main (25 files, +1065/-225) and ran the complete test suite locally (829 passed, 24 skipped). All CI checks on this head are green. No blocking findings.

What this PR does

Introduces worker.py as the new catalog entrypoint for github-issue-to-pr, running issue delivery inside the conversation provisioned by the Automation Service's selected agent profile. The existing main.py remains the source of implementation prompts, Git helpers, and PR publication — worker.py adds readiness selection, dependency gating, PR-revision detection, and conditional base-update handling on top of it. A new shared github_client.py provides the GitHubRepository transport and run_repositories driver used by all four GitHub automations.

Areas examined

worker.py (new, 259 lines)

• IssueToPR extends the shared GitHubRepository transport — no duplication of GitHub API code.
• run(): lane partitioning uses exact type(...) is int checks (correctly rejects bool), modulo-based issue ownership, and priority ordering (priority:high first, then issue number). PRs with the review label are skipped so a pending reviewer doesn't block the lane.
• Base SHA is fetched once per batch (not per PR).
• update-branch handles 409 (head changed → skip with JSON log line and continue to next PR) and 422 (conflict → developer revision with conflict context in the prompt). Other HTTP errors are re-raised.
• implement(): interrupts a stuck agent, polls for a stopped state with a 30s deadline, writes a redacted checkpoint.json via workflow._redact, and posts a generic reason (no error details or credentials leak into public comments).
• publish(): zero-commit revisions on an existing PR request another review via review_label without pushing; zero-commits on a new issue raises RuntimeError unless a checkpoint exists. PR body routes through workflow._pull_request_body → _with_ai_disclosure.

main.py changes

• _prepare_repository now skips git checkout -b when branch == base_branch (revision path checks out the existing branch directly).
• _build_implementation_prompt gains publish_pr and github_access_instructions keyword-only params with additive defaults. When publish_pr=False, the agent summarizes instead of pushing/opening the PR. Access instructions are parameterized so the worker can pass the profile-scoped token name.
• The revision path passes checkout_branch as base_branch to the prompt, so the workspace description is factually correct.

github_client.py (new, 204 lines)

• Shared github_request / github_paginate with a 90s timeout and 100-page pagination cap.
• GitHubRepository base class: validates owner/repo and token-secret name against safe regexes, caches base_branch, provides gh, shell, comment (with AI disclosure), open_issues, statuses, dependencies_complete, and gh_pages.
• run_repositories driver: processes repos sequentially, isolates failures per-repo, and raises RuntimeError listing all failed repos.

manifest.json

• Entrypoint → worker.py, version → 1.1.0, timeout → 3000s. worker.py and github_client.py added to bundle files. New githubTokenSecret form field and agentProfiles feature requirement.

Tests (264 lines, 16 cases)

• Covers canonical publication reuse, no-code revision + review-label request, lane selection by priority (parametrized across main/release and pending-review states), failed-review revision with live feedback, base refresh independence from acceptance, checkpoint stop requirement (both outcomes), credential redaction in checkpoints vs. public comments, and base-update with head-change race. All use real code paths through the bundle helper with mocked GitHub/SDK boundaries.

Shared foundation tests (73 lines)

• Repository run isolation, pagination type safety, dependency completion gating, and 403 fail-closed behavior.

Installation tests (64 lines)

• Verifies github_client.py is included in all four GitHub skills and that worker.py imports cleanly.

Catalog integrity

• bundle-index.js and index.js synchronized; all catalog sync, schema, and skill installation tests pass.

Security

• Credentials are redacted via workflow._redact before writing checkpoints (verified by test_checkpoint_keeps_error_details_out_of_public_comments).
• The token is referenced by name (--github-token-secret), never embedded in the bundle.
• github_instructions tells the agent to use GH_TOKEN=$TOKEN_NAME and never print the value.
• PR bodies include the AI disclosure via _pull_request_body → _with_ai_disclosure.
• GitHubRepository.__init__ validates both the repository name and token-secret name against safe regexes before use.

No blocking findings. Approving.

Generated by OpenHands AI on behalf of the user.

Review of head 0f1a330 — ✅ Approved

I reviewed the full diff against main (9 files, +661/−46) and ran the complete focused test suite (21 passed, 191 catalog integrity tests passed). No blocking findings.

What this PR does

Introduces worker.py as the new catalog entrypoint for github-issue-to-pr, running issue delivery inside the conversation provisioned by the Automation Service's selected agent profile. The existing main.py remains the source of implementation prompts, Git helpers, and PR publication — worker.py adds readiness selection, dependency gating, PR-revision detection, and base-update handling on top of it.

Areas examined

worker.py (new, 258 lines)

• IssueToPR extends the shared GitHubRepository transport — no duplication of GitHub API code.
• run(): lane partitioning uses exact type(...) is int checks (correctly rejects bool), modulo-based issue ownership, and priority ordering (priority:high first, then issue number). PRs with the review label are skipped so a pending reviewer doesn't block the lane.
• Base SHA is fetched once per batch (not per PR), satisfying earlier review feedback.
• update-branch handles 409 (head changed → skip to next run) and 422 (conflict → developer revision with conflict context in the prompt). Other HTTP errors are re-raised.
• implement(): interrupts a stuck agent, polls for a stopped state with a 30s deadline, writes a redacted checkpoint.json, and posts a generic reason (no error details or credentials leak into public comments).
• publish(): zero-commit revisions on an existing PR request another review via review_label without pushing; zero-commits on a new issue raises RuntimeError unless a checkpoint exists.

main.py changes

• _prepare_repository now skips git checkout -b when branch == base_branch (revision path checks out the existing branch).
• _build_implementation_prompt gains publish_pr and github_access_instructions keyword-only params. When publish_pr=False, the agent summarizes instead of pushing/opening the PR — the coordinator owns publication. Access instructions are parameterized so the worker can pass the profile-scoped token name.

manifest.json

• Entrypoint → worker.py, version → 1.1.0, timeout → 3000s (matches the 2400s agent run + overhead). worker.py added to bundle files. New githubTokenSecret form field and agentProfiles feature requirement.

Tests (264 lines, 16 cases)

• Covers canonical publication reuse, no-code revision + review-label request, lane selection by priority (parametrized across main/release and pending-review states), failed-review revision with live feedback, base refresh independence from acceptance, checkpoint stop requirement (both outcomes), credential redaction in checkpoints vs. public comments, and base-update with head-change race. All use real code paths through the bundle helper with mocked GitHub/SDK boundaries.

Catalog integrity

• 191 catalog tests pass — the bundle index, manifest, and skill catalog are consistent.

Security

• Credentials are redacted via workflow._redact before writing checkpoints; raw error details and the token never appear in public comments.
• The token is referenced by name (--github-token-secret), never embedded in the bundle.
• github_instructions tells the agent to use GH_TOKEN=$TOKEN_NAME and never print the value.

Minor notes (non-blocking)

• type(lane) is not int is intentional (rejects bool), though a one-line comment would help future readers.
• One long line in SKILL.md's "Explicit Depends on" paragraph — cosmetic only.

No blocking findings. Approving.

Generated by OpenHands AI on behalf of the user.

Review of head b486ebb -- Approved

Reviewed the full diff against main (9 files, +661/-46) and the incremental diff from 3e39069 to b486ebb (6 files, +105/-367). Ran the focused test suite (4 developer delivery tests pass; 906 passed, 24 skipped overall). No material findings.

What changed in this head

The second commit adds per-issue failure isolation and removes obsolete lifecycle documentation:

1. worker.py -- new _try_submit() wraps _submit() in try/except, logging the repository and issue number on failure and continuing to the next eligible issue rather than aborting the whole run. Both call sites in run() (revision submissions and new-issue submissions) now use _try_submit. The broad except Exception is appropriate here: the scanner's job is to fan out idempotent subject turns, and one transient API or network failure must not block unrelated issues.

2. tests/test_github_developer_delivery.py -- new test_developer_continues_after_one_submission_fails verifies that a RuntimeError on the first issue's submission does not prevent the second issue from being submitted, and that both subject keys are attempted in priority order.

3. SKILL.md -- 361 lines of obsolete self-managed lifecycle documentation (manual script rewriting, uploads, state files, conversation polling, clone/checkpoint management, 11-step setup workflow) replaced with 62 lines accurately describing the delegation model: the scanner selects work and writes the agent instruction; Automation owns conversation creation, rescheduling, and cleanup. The new content matches the actual worker.py entrypoint and catalog manifest.

Areas examined

• Failure isolation correctness: _try_submit catches at the per-issue boundary, so a failure after the base SHA fetch (which is shared) still allows subsequent issues to use it. The idempotency key (issue_number:updated_at or issue_number:revision_sha) means a failed submission is safely retried on the next scan.
• Test coverage: the new test directly exercises the isolation path with a real exception from submit_subject_turn, not a mock of _try_submit itself. The existing tests (priority fan-out, failed-review revision, open-PR skip) still pass unchanged.
• Documentation accuracy: the new SKILL.md correctly describes the four selection steps, the stable subject key (repository_id:issue:number), the review-label flow, and the credential model. No references to removed lifecycle code remain.
• Manifest and bundle: unchanged from the prior approved head; entrypoint: python3 worker.py, bundle files, and config schema are consistent.

Note on version tests

Two test_version_alignment tests fail locally because the installed openhands_extensions package resolves to a different workspace checkout (OpenHands_extensions_572/repo) that still reports 0.20.0. The PR correctly bumps all three version sources (package.json, _version.py, .release-please-manifest.json) to 0.21.0 in lockstep. This is an environment artifact, not a PR defect.

Generated by OpenHands AI on behalf of the user.